fix: prevent zombie instances on app restart#23757
Conversation
Disconnect connectionManager before daemon stop in performRestart() to prevent autoWakeIfAssistantDied() from fighting with the shutdown. Add isRestarting flag so applicationShouldTerminate returns .terminateNow, skipping the redundant second cli.stop() and fragile async MainActor.run dispatch that could leave the process as a zombie. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| self?.isRestarting = false | ||
| // Reconnect SSE and health checks so the app doesn't stay | ||
| // in a disconnected state after a failed relaunch attempt. | ||
| // (Same pattern as performRetireAsync()'s cancel path.) | ||
| Task { @MainActor [weak self] in | ||
| try? await self?.connectionManager.connect() | ||
| } |
There was a problem hiding this comment.
🔴 isRestarting = false written off the main actor in NSWorkspace completion handler
AppDelegate is @MainActor, so isRestarting is main-actor-isolated. However, in the error path of performRestart(), the NSWorkspace.shared.openApplication completion handler runs on a background serial queue (not the main actor). The assignment self?.isRestarting = false at line 176 writes this main-actor-isolated property from a non-isolated context — a data race.
On Apple Silicon (ARM64, weak memory model), the write may never become visible to the main actor. If the restart fails and isRestarting remains stale true, a subsequent app quit triggers applicationShouldTerminate (AppDelegate.swift:753) which reads isRestarting, sees true, and returns .terminateNow — bypassing the vellumCli.stop() cleanup. The reconnection on line 180-182 is correctly wrapped in Task { @MainActor ... } but the flag reset is not.
| self?.isRestarting = false | |
| // Reconnect SSE and health checks so the app doesn't stay | |
| // in a disconnected state after a failed relaunch attempt. | |
| // (Same pattern as performRetireAsync()'s cancel path.) | |
| Task { @MainActor [weak self] in | |
| try? await self?.connectionManager.connect() | |
| } | |
| Task { @MainActor [weak self] in | |
| self?.isRestarting = false | |
| // Reconnect SSE and health checks so the app doesn't stay | |
| // in a disconnected state after a failed relaunch attempt. | |
| // (Same pattern as performRetireAsync()'s cancel path.) | |
| try? await self?.connectionManager.connect() | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
* fix: prevent zombie instances on app restart (#23757) * fix: prevent zombie instances on app restart Disconnect connectionManager before daemon stop in performRestart() to prevent autoWakeIfAssistantDied() from fighting with the shutdown. Add isRestarting flag so applicationShouldTerminate returns .terminateNow, skipping the redundant second cli.stop() and fragile async MainActor.run dispatch that could leave the process as a zombie. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reset isRestarting flag on relaunch failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reconnect connectionManager on restart relaunch failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: move isRestarting flag to success path to avoid premature terminateNow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
connectionManagerbefore daemon stop inperformRestart()to preventautoWakeIfAssistantDied()from fighting the shutdown (same pattern asperformRetireAsync())isRestartingflag soapplicationShouldTerminatereturns.terminateNow, skipping the redundant secondcli.stop()and fragile asyncMainActor.rundispatch that could leave the process as a zombieTest plan
.terminateLaterpathFixes #23756.
🤖 Generated with Claude Code